feat: smart PCH rebuild, #include/import completion, rapid-edit robustness - #394

src/syntax/scan.cpp (1)

483-491: CRLF line endings may cause false positives.

The split('\n') approach doesn't strip \r from Windows-style line endings. If a user types #include\r\n (incomplete directive with CRLF), after_keyword would be "\r" instead of empty, causing the check at line 518 to incorrectly treat it as complete (macro case).

Consider trimming the line on both sides or explicitly handling \r:

🔧 Suggested fix

     while(!preamble.empty()) {
         auto [line, rest] = preamble.split('\n');
         preamble = rest;

-        auto trimmed = line.ltrim();
+        auto trimmed = line.ltrim().rtrim("\r");

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/server/master_server.cpp (1)

683-709: ⚠️ Potential issue | 🟠 Major

scan(text) does not populate modules — this code path is ineffective.

As noted in a previous review, the lexer-based scan() function only extracts #include directives and module declarations (module_name, is_interface_unit). It does not handle import statements — those are only captured by scan_precise() through its PreciseScanPPCallbacks::moduleImport() callback.

The ScanResult::modules vector (see src/syntax/scan.h:50) is only populated by scan_precise(), not by scan(). Therefore, scan_result.modules will always be empty here, making the entire iteration block unreachable dead code.

To discover import statements from the in-memory buffer, either:

1. Use scan_precise() (requires compilation arguments, more expensive)
2. Extend scan() to extract cxx_import_decl / cxx_export_import_decl directives

,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/server/master_server.cpp` around lines 683 - 709, The current buffer-scan
uses scan(text) but ScanResult::modules is only filled by scan_precise(), so the
loop is dead; fix by either (A) switching this path to call scan_precise(...)
with the appropriate compilation arguments and then use the returned
ScanResult.modules (leveraging PreciseScanPPCallbacks::moduleImport), or (B)
extend the lightweight scan(...) implementation to also recognize C++20
import/export import directives (cxx_import_decl / cxx_export_import_decl) and
populate ScanResult::modules so the existing logic (checking path_to_module,
pcm_paths, compile_graph->compile_deps(pid)) works; update references in this
block to use the chosen function (scan_precise or the enhanced scan) and ensure
ScanResult::modules is actually filled before iterating.

src/syntax/scan.cpp (1)

479-491: Missing word-boundary check for #include/#import directive keywords.

The function checks directive.starts_with("include") or directive.starts_with("import") without verifying a word boundary, meaning a hypothetical directive like #includefoo would be incorrectly treated as an include directive. While this is an unlikely edge case in practice, it's inconsistent with the word-boundary check applied for C++20 import/export keywords in is_preamble_complete (lines 521-524).

Also, the keyword length calculation at line 482/489 assumes "import" (6 chars) or "include" (7 chars), but #import in Objective-C uses 6 chars. If a #include_next directive were passed here, the offset would be wrong.

🔧 Suggested improvement for robustness

 static bool is_include_directive_complete(llvm::StringRef directive) {
+    // Determine keyword and its length
+    llvm::StringRef keyword;
+    if(directive.starts_with("include_next") && 
+       (directive.size() == 12 || !llvm::isAlnum(directive[12]))) {
+        keyword = "include_next";
+    } else if(directive.starts_with("include") && 
+              (directive.size() == 7 || !llvm::isAlnum(directive[7]))) {
+        keyword = "include";
+    } else if(directive.starts_with("import") && 
+              (directive.size() == 6 || !llvm::isAlnum(directive[6]))) {
+        keyword = "import";
+    } else {
+        return true;  // Not a recognized include/import directive
+    }
+
     if(directive.contains('"')) {
-        auto after_keyword = directive.drop_front(directive.starts_with("import") ? 6 : 7);
+        auto after_keyword = directive.drop_front(keyword.size());
         return after_keyword.count('"') >= 2;
     }
     if(directive.contains('<')) {
         return directive.contains('>');
     }
     // No " or < — might be a macro (`#include` FOO) or just incomplete (`#include` ).
-    auto after_keyword = directive.drop_front(directive.starts_with("import") ? 6 : 7).ltrim();
+    auto after_keyword = directive.drop_front(keyword.size()).ltrim();
     return !after_keyword.empty();
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/syntax/scan.cpp` around lines 479 - 491, is_include_directive_complete
currently uses directive.starts_with("import")/("include") without ensuring a
word boundary and then hardcodes lengths 6/7; update
is_include_directive_complete to first detect which keyword actually matches
("import" vs "include") and verify a word boundary by checking the character
after the keyword is end-of-string, whitespace, '"' or '<' (so "#include_next"
won't be treated as "include"); compute the keyword length from that matched
keyword and use it when computing after_keyword (instead of always using 6/7) so
Objective-C "#import" and other edge cases are handled correctly.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes